Conversation
amao9098
commented
Oct 24, 2024
amao9098
commented
- The setup should always be adding the public key even if the file exists - causes issues in external since the file already exists so new keys are not being added
PERFTEST.PS1
Outdated
| [parameter(Mandatory=$true)] [string] $SrcIp, | ||
| [parameter(Mandatory=$true)] [string] $DestIp, | ||
| [parameter(Mandatory=$true)] [ValidateScript({Test-Path $_ -PathType Container})] [String] $OutDir = "", | ||
| [ValidateSet("Default", "Azure", "Detail", "Max", "Container")] |
There was a problem hiding this comment.
Did you mean to remove this line?
There was a problem hiding this comment.
Yes, this prevents adding new configs for one off tests.
SetupTearDown.ps1
Outdated
| Write-Host "`nTrusted admin keys already exist" | ||
| } | ||
| Write-Host "`nAdd the AuthorizedKey as a trusted admin key" | ||
| Add-Content -Force -Path "$env:ProgramData\ssh\administrators_authorized_keys" -Value "`n$authorizedKey" |
There was a problem hiding this comment.
Do you know if it will cause problems if we add the same key multiple times? I think after this change we may add the same key multiple times to the authorized keys file. Maybe it would be better to see if the key is present in the file before adding it.
Also, are you hitting this during manual or automated end-to-end test passes? Since we're recreating the containers for every test, you shouldn't hit this issue for the full test passes.
There was a problem hiding this comment.
It’s for the external setup. I can check if the passkey already exists for better code cleanup.
| if (-NOT (Get-Content -Path "$env:ProgramData\ssh\administrators_authorized_keys" | ForEach-Object{$_ -match $authorizedKey})) { | ||
| Write-Host "`nAdd the AuthorizedKey as a trusted admin key" | ||
| Add-Content -Force -Path "$env:ProgramData\ssh\administrators_authorized_keys" -Value "$authorizedKey" | ||
| Add-Content -Force -Path "$env:ProgramData\ssh\administrators_authorized_keys" -Value "`n$authorizedKey" |
There was a problem hiding this comment.
Do you need the `n? I think Add-Content will add a new line by default.
There was a problem hiding this comment.
Looks like the `n is still here.
SetupTearDown.ps1
Outdated
|
|
||
| if (-NOT (Test-Path "$env:ProgramData\ssh\administrators_authorized_keys")) | ||
| { | ||
| if (-NOT (Get-Content -Path "$env:ProgramData\ssh\administrators_authorized_keys" | ForEach-Object{$_ -match $authorizedKey})) { |
There was a problem hiding this comment.
I don't think this will work the way you want it to. If Get-Content returns more than one line, the result of the Get-Content | Foreach {} is going to be an array and an array is always considered "true" when interpreted as bool. So, this line will work if the authorized_keys file is empty or has one line, but will always skip adding the key if there is more than one key in the file, even if the key being checked isn't in there.
You could do something like this:
$isPresent = $false
PS C:\Users\marsam.REDMOND> Get-Content | foreach { $isPresent = $isPresent -OR $_ -match $authorizedKey }
if (-NOT $IsPresent)
{...}
There was a problem hiding this comment.
modified the statement so that it just checks the string!
MarSamMS
left a comment
MarSamMS
left a comment
There was a problem hiding this comment.
Approve, but left one comment
| if (-NOT (Get-Content -Path "$env:ProgramData\ssh\administrators_authorized_keys" | ForEach-Object{$_ -match $authorizedKey})) { | ||
| Write-Host "`nAdd the AuthorizedKey as a trusted admin key" | ||
| Add-Content -Force -Path "$env:ProgramData\ssh\administrators_authorized_keys" -Value "$authorizedKey" | ||
| Add-Content -Force -Path "$env:ProgramData\ssh\administrators_authorized_keys" -Value "`n$authorizedKey" |
There was a problem hiding this comment.
Looks like the `n is still here.
2 participants
